Preserve legacy command readback and refuse ambiguous request keys - #1202

Security review at exact head 4c02773582cd59192dd002b380a6d41b4fb3ec73: one missing authority vector is blocking PASS; one response assertion should be added with it.

The core correction is sound in source:

• Authorization/namespace instance lookup still precedes the receipt lookup; the lookup remains inside the serialized transition transaction and before run-state gates.
• Signal/update key spaces and workflow-instance scope stay separate. The query considers both hashed and legacy raw representations, rejects multiple candidates, then requires exact stored bytes according to whether an intent row exists. One collation-only match, a token-shaped legacy raw key, a modern row addressed through its token bytes, and two-row raw/token collisions all fail closed without exposing a candidate.
• Legacy readback keeps the original command/run/status and marks retained:true,intent_verified:false; changed names or explicit run targets are refused before admission. Argument equality is not invented. Closure, restart, continuation and concurrent retry vectors keep table counts/history fixed.
• Ambiguity returns a nonretryable typed 409 without a selected command. Modern CommandIntent v1 and capability bytes are unchanged.

Blocking missing evidence: updates.rs moved claim_external_in() behind keyed.is_none() so a retained retry does not take ownership of the retry's external payload reference. That is a customer-resource authority change and is explicitly claimed in the PR handoff, but no changed test uses an external reference. Add an all-backend vector that uploads an original update input and a distinct retry reference, retries under the retained key, and proves:

1. the original receipt/command/history is returned unchanged;
2. the retry's reference remains unclaimed/available to its owner;
3. no command, intent, history, task, or registry ownership row changes;
4. both a verified same-intent receipt and a legacy unverified receipt are covered if those paths differ.

The existing external-payload tests already expose an unclaimed() count suitable for this boundary. With the same update, assert retryable:false on every request_id_ambiguous shape; the source sets it, but the public nonretryable contract is currently untested.

Please send the corrected exact SHA and all-backend results. I found no other source blocker in the legacy identity, target, collision, readback, or zero-write paths.

Security follow-up is implemented: a real external-payload upload test covers both verified and legacy unverified update receipts. It uploads distinct original/retry references with the same typed value, checks the original response apart from the explicit retention/intent markers, retains one unclaimed retry reference, and compares complete before/after rows for commands, intents, history, tasks, updates, runs, instances and the payload registry. All ambiguity shapes now assert retryable:false.

The existing schema-upgrade qualifications now seed raw signal/update command keys before the prior schema's upgrade and verify those exact keys/run identities survive without fabricated intent rows. This includes MySQL/MariaDB's interrupted-DDL resume path. No schema or migration bytes changed.

Local SQLite command-key coverage: eight tests passed before this last test-only extension. The SQLite upgrade unit test and the extended external-reference test are compiling/running now; the full all-backend Actions result remains required. Initial local lib-test compilation exceeded a 4 GiB container cap; the current bounded 8 GiB run has fresh host headroom. No passing upgrade/external result is claimed yet.

Focused source verification at exact head 2481c5007acb317be6328d98444c3a53e92784b6 (2026-11-13), after the Security-requested authority vector:

The command-key tests include restart, run close, continue-as-new, concurrent retries, changed names/explicit targets, exact key casing, and raw/token ambiguity for both command types. Every ambiguity shape now asserts retryable:false.

The ownership test uploads a retained original and a distinct carrier for the same decoded typed value. Modern and legacy retries return the original receipt, leave the retry reference unclaimed, and preserve complete stored rows for commands, intents, history, tasks, updates, runs, instances and the payload registry. This covers the authority change in updates.rs.

Runner: isolated Linux amd64 containers, pinned Rust toolchain index sha256:114c7a4425406451c2866b6aafe69fe29b1b298832db1277d411ac73c82d04d6, two Cargo jobs and two test threads. DB image config IDs: MySQL sha256:7dcddc01f13bab2f15cde676d44d01f61fc9f99fe7785e86196dfc07d358ae2b, MariaDB sha256:7db29378d4fdab73f8123bbc2b48905c90d1a4b00cf848b028f1e81e623257f2, PostgreSQL sha256:3540c0362c809df084ffac0ae69fe794515f9de92692f02178bcdce0ef0302ac. Tests ran sequentially, without production credentials or published ports. Exact owned DB containers, runners, anonymous DB volumes and the internal network were removed.

Two setup limitations were resolved without changing product behavior: lib-test compilation exceeded its initial 4 GiB cap and passed at an 8 GiB ceiling with measured headroom; the first database attempt used a local-socket readiness probe during MySQL initialization and was runner-blocked before admission tests, then passed with TCP readiness. The first external fixture reused an identical upload's reference; the corrected fixture uses distinct stored bytes that decode to the same typed value.

These are source regression/upgrade results, not published-artifact conformance or a full CI matrix pass. Current-head normal Actions and exact-head Security PASS remain required. The 3.9.0 promotion/Release hold remains, and the correction will publish as 3.9.1 before installed SDK/CLI lost-response qualification.

Security PASS for exact head 2481c5007acb317be6328d98444c3a53e92784b6, subject to the repository's still-running required checks completing green.

I re-read the corrected source and the new controls after the prior authority finding:

• retained_command() looks up both hashed and legacy raw representations before run-state gates, rejects multiple candidates, and validates the exact stored representation according to whether an intent row exists. A collation-only match, token-shaped legacy raw key, modern row addressed as a raw key, or raw/token collision fails closed as nonretryable 409 request_id_ambiguous without selecting a receipt.
• Legacy signal/update readback preserves the original command/run/status, reports retained:true,intent_verified:false, never invents argument equality, and refuses a changed name or explicit run target.
• Update external-payload ownership is now claimed only for a genuinely new admission. Modern verified and legacy unverified retained retries return the original receipt while the retry's distinct reference remains owned by its uploader.
• The authority vector compares complete stored rows for commands, intents, history, tasks, updates, runs, instances and the external-payload registry, rather than relying on counts.
• Prior-schema controls preserve exact raw signal/update keys and run identities without fabricating intent rows. SQLite, MySQL, MariaDB and PostgreSQL each pass the eight command-key controls, modern+legacy external-reference ownership control and their prior-schema upgrade control; MySQL/MariaDB also pass interrupted-DDL resumption.
• Documentation accurately distinguishes unverified legacy readback from verified CommandIntent v1 equality and instructs callers not to retry ambiguous keys automatically.

No remaining source or authority blocker was found. The normal Actions matrix is still running, so this is not authorization to merge, promote or publish on a failed/incomplete gate. Merge only at this exact head after every required check is green; then publish the correction under the new immutable 3.9.1 identity and retain the existing 3.9.0 alias/final-Release hold until its release qualification is complete. Any head change requires counter-review.

Run 43402885487 found three expectation sites that still treated retained receipts as byte-identical to first admission:

• The structural-limit rejection test needed the explicit raw-key readback markers (retained:true,intent_verified:false).
• The terminal-update isolation test needed the verified duplicate markers on the unrelated active run's original receipt.
• Corpus-b's update-validation contract needed explicit native retained-approval/rejection intent metadata. This was an expectation difference, not the Corpus-b flake: large-command-batches overflow polls exhaust PHP reference lock-pressure retries #1194 lock-pressure flake.

The successor changes tests/comparator expectations only. Runtime, schemas and the authority boundary are byte-identical to Security-passed 2481c5007acb317be6328d98444c3a53e92784b6. Both affected Rust tests now pass locally. The native-only behavior is registered as retained-command-intent-readback; all original receipt fields stay exact, and published PHP recordings/frozen fixtures are unchanged. The 28 update-validation/divergence unit tests pass, including wrong/missing intent-proof flags and metadata appearing on an original admission.

Fresh current-head CI and Security counter-review remain required; the prior source PASS is not being reused for the new head. The 3.9.0 hold remains.

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Corrected comparator-only successor: cb7158a. The recorder supplies its implementation target as a separate checkObservation argument; the prior check incorrectly read observation.target. The target is now forwarded explicitly. A regression through the real recorder contract entry point catches that wiring error. All 299 comparator tests pass. A clean SQLite source build also passes validators_approve_and_reject_updates_before_acceptance with explicit true/true retained approval and true/false raw-key rejection assertions. Runtime and schema bytes remain identical to the Security-reviewed 2481c50 head. Current-head required CI and Security counter-review remain gates; 3.9.0 promotion is still held. Run43405494921 also exposed a distinct MariaDB worker_poll_fence registration pool_timeout (HTTP503 instead of201); I am investigating that separately, without accepting a rerun as a correction.

Security counter-review: source/test delta is clean at exact cb7158a02c4fab6ef09d325425e5d2ea1c1a9469; merge/promotion HOLD remains pending fresh required CI.

Relative to the previously passed runtime/schema head 2481c500, this successor changes tests and parity validation only. The final correction now passes the recording target explicitly from checkObservation(..., target) into update-validation checking, rather than reading a nonexistent probe-observation target. The caller regression proves Rust accepts the explicit retained-intent markers while the PHP fixture does not silently inherit the divergence. The native update test independently requires retained approval true/true and legacy/raw-key rejection true/false.

I independently ran the two affected Node contract suites: 292 tests passed. Runtime and schema remain byte-identical to 2481c500.

This is not a merge or 3.9.1 promotion PASS yet: current-head CI is still running, and the separately observed MariaDB worker_poll_fence 503/pool-timeout result must be resolved or proven unrelated with a clean required result. Any source/head change needs counter-review.